class-declaration-abstractness-changed #974
Conversation
| @@ -54,11 +54,17 @@ const classDeclarationTypeChanged: ComparerFactory = (context) => ({ | |||
| if (aType === bType) { | |||
| return; | |||
| } | |||
| context.report({ | |||
There was a problem hiding this comment.
The existing type-change report shouldn't be removed. Instead, the if statement above should be inverted (aType !== bType or whatever) and the report should be put in there.
| const changeSeverity = isAbstract(a) ? 'minor' : 'major'; | ||
| context.report({ | ||
| key: 'class-declaration-abstractness-changed', | ||
| message: `The class "${a.getName()}" changed from ${changeType} (${changeSeverity} change).`, |
There was a problem hiding this comment.
The severity can't simply be expressed in the message but must be defined here:
You'll need to create two different keys: one for abstract to concrete, and one for concrete to abstract.
There was a problem hiding this comment.
I have added the rules, and change the severity to changeKey.
The pull request is ready for review.
please let me know if any changes need.
|
It looks okay to me, but I'm not part of the Concerto team so they'll have to review it. I expect they would like to see a test, for example. I think you can search for the other result codes (keys) and find where the existing tests are. |
|
Thanks for the suggestion! I’ll look into searching for the result codes
and check the existing tests.
…On Sun, Dec 22, 2024, 11:31 PM DS-AdamMilazzo ***@***.***> wrote:
It looks okay to me, but I'm not part of the Concerto team so they'll have
to review it. I expect they would like to see a test, for example. I think
you can search for the other result codes (keys) and find where the
existing tests are.
—
Reply to this email directly, view it on GitHub
<#974 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BKJ2ERKOFVNQBUFEJAXYV3D2G35A5AVCNFSM6AAAAABUAZFFHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGUZTSNJSHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hi AdamMilazzo,
I hope you're doing well! I’m really interested in contributing to the
Accord project, especially for GSoC 2025. I’ve started exploring the
codebase and would love to begin contributing by fixing bugs or adding
small features. Could you recommend any tasks or open issues that I can
start with?
I’d also appreciate any advice on how to get more involved early on. How
can I connect with you or other mentors to discuss potential project ideas
for next year? Additionally, how can I find the listed projects for the
Accord organization for GSoC 2025?
Also, I was wondering how I can connect with Accord members in the forums
or other community channels. Any tips on how to get in touch with other
contributors or mentors there would be great!
Thank you for your time and support! Looking forward to hearing from you.
Best regards,
B Mamatha
***@***.***
On Mon, Dec 23, 2024, 12:08 AM Mamatha Bandi ***@***.***>
wrote:
… Thanks for the suggestion! I’ll look into searching for the result codes
and check the existing tests.
On Sun, Dec 22, 2024, 11:31 PM DS-AdamMilazzo ***@***.***>
wrote:
> It looks okay to me, but I'm not part of the Concerto team so they'll
> have to review it. I expect they would like to see a test, for example. I
> think you can search for the other result codes (keys) and find where the
> existing tests are.
>
> —
> Reply to this email directly, view it on GitHub
> <#974 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BKJ2ERKOFVNQBUFEJAXYV3D2G35A5AVCNFSM6AAAAABUAZFFHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGUZTSNJSHE>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
Hi @Mamatha1718 I might also want to do a DCO signoff for your commits. |
| @@ -53,6 +53,8 @@ export const defaultCompareConfig: CompareConfig = { | |||
| 'optional-property-added': CompareResult.PATCH, | |||
| 'required-property-removed': CompareResult.MAJOR, | |||
| 'optional-property-removed': CompareResult.MAJOR, | |||
| 'class-declaration-abstract-to-concrete': CompareResult.MINOR, | |||
| 'class-declaration-concrete-to-abstract': CompareResult.MAJOR, | |||
There was a problem hiding this comment.
How do we decide if the conversions results in a major/minor bump? Is there some sort of modelling convnetion for this?
There was a problem hiding this comment.
It's major if existing data can become invalid (i.e. it's not backward-compatible). Otherwise, it's minor or patch. I don't know how they decide between those two. @mttrbrts ?
|
Title
See our DEVELOPERS guide below:
https://github.com/accordproject/concerto/blob/main/DEVELOPERS.md
Closes #974
Summary
This pull request introduces abstractness checks for class declarations to enhance compatibility and versioning analysis.
Changes
=> Added logic to compare
isAbstractproperty in class declarations.=> Retained existing type-change reports for class declarations.
=> Introduced new keys:
=>
class-declaration-abstract-to-concrete: Classified as a minor change.=>
class-declaration-concrete-to-abstract: Classified as a major change.=>Updated rules in
compare-config.tsto reflect the above changes.+Flags
=> Ensure the abstractness change handling aligns with other compatibility rules.
=> Breaking changes introduced by
concrete-to-abstractmodifications must be validated thoroughly.Related Issues
Author Checklist
--signoffoption of git commit.mainfromfork:branchname